mmc: Fix MMC request issuing race condition to prevent stall#7486
Conversation
|
Have you experienced a situation with an A2 SD card where you got such a hang, and this was likely to be the cause? |
Hi, sorry, I have not. I have only ever tested on eMMCs on a CM5 device, which this issue can be easily reproduced running stress-ng's directory test. |
|
Please provide the stress-ng commandline that provokes the bug. |
run_disk_stress_ng.sh |
|
That script has been running for over an hour with no fails - CM5, 16GB eMMC, 2GB RAM, Pi OS Trixie with 6.18.34+rpt-rpi-2712 (latest apt kernel), |
Oh I forgot, you need to enable |
|
Mounting as discard is very not-recommended as it kills performance (and flash endurance), but does provoke the bug in short order. It is underflowing, this patch fixes that. I'd prefer declaring both variables as ints and explicitly warning on underflow in case there's another situation that can cause it. Also for consistency use the bool in the test for BLK_STS_RESOURCE. I think I've pushed the right buttons to update your branch, please review/test. |
Hi, I have reviewed and retested, they look good and still work. Thank you for the changes! |
Posted write tracking introduced in the commit below referenced the request in the submission path after it had been issued, racing with re-use of the request and potentially causing underflow of the pending write count. The count is signed but was compared against the unsigned posted write limit, so such an underflow would wrap to a large value and wrongly throttle further writes (hang entire system). Fixes: e6c1e86 ("mmc: restrict posted write counts for SD cards in CQ mode") Co-developed-by: Jonathan Bell <jonathan@raspberrypi.com> Signed-off-by: Jonathan Bell <jonathan@raspberrypi.com> Signed-off-by: Yufeng Gao <yufeng.gao@canonical.com>
2 participants
Posted write tracking introduced in the commit below referenced the request in the submission path after it had been issued, racing with re-use of the request and potentially causing underflow of the pending write count. The count is signed but was compared against the unsigned posted write limit, so such an underflow would wrap to a large value and wrongly throttle further writes (hang entire system).
Fixes: e6c1e86 ("mmc: restrict posted write counts for SD cards in CQ mode")
This fix is the same as ef0aded, but for the request issuing path. It additionally fixes the unsigned vs signed integer comparison, which actually causes stalls when
pending_writesdoes underflow to negative for whatever reason as it is promoted to unsigned 0xFFF... or some huge unsigned integer, causing the comparison to always be true andBLK_STS_RESOURCEalways returned.